-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix MutVisitor's default implementations to visit Stmt's and BinOp's spans #133784
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
r? @chenyukang rustbot has assigned @chenyukang. Use |
rustbot
added
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Dec 3, 2024
tjallingt
reviewed
Dec 3, 2024
compiler/rustc_ast/src/mut_visit.rs
Outdated
Comment on lines
1631
to
1635
ExprKind::Binary(binop, lhs, rhs) => { | ||
vis.visit_span(&mut binop.span); | ||
vis.visit_expr(lhs); | ||
vis.visit_expr(rhs); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
ExprKind::Binary(binop, lhs, rhs) => { | |
vis.visit_span(&mut binop.span); | |
vis.visit_expr(lhs); | |
vis.visit_expr(rhs); | |
} | |
ExprKind::Binary(binop, lhs, rhs) => { | |
vis.visit_expr(lhs); | |
vis.visit_expr(rhs); | |
vis.visit_span(&mut binop.span); | |
} |
To match the order defined by #126993
r? compiler-errors @bors r+ |
bors
added
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Dec 3, 2024
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 4, 2024
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#133651 (Update `NonZero` and `NonNull` to not field-project (per MCP#807)) - rust-lang#133764 (rustdoc: Rename `set_back_info` to `restore_module_data`.) - rust-lang#133784 (Fix MutVisitor's default implementations to visit Stmt's and BinOp's spans) - rust-lang#133798 (stop replacing bivariant args with `'static` when computing closure requirements) - rust-lang#133804 (Improve code for FileName retrieval in rustdoc) - rust-lang#133817 (Use `eprintln` instead of `println` in bootstrap/compiletest/tidy) Failed merges: - rust-lang#133810 (remove unnecessary `eval_verify_bound`) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 4, 2024
Rollup merge of rust-lang#133784 - dtolnay:visitspans, r=compiler-errors Fix MutVisitor's default implementations to visit Stmt's and BinOp's spans The `Stmt` case is a bug introduced almost certainly unintentionally by rust-lang#126993. The code _used_ to visit and mutate `span` correctly, but got changed as follows by that PR. Notice how `span` is **copied** into the output by `|kind| Stmt { id, kind, span }` which happens after the mutation in the correct code (red) and before the mutation in the incorrect code (green). ```diff pub fn noop_flat_map_stmt<T: MutVisitor>( Stmt { kind, mut span, mut id }: Stmt, vis: &mut T, ) -> SmallVec<[Stmt; 1]> { vis.visit_id(&mut id); - vis.visit_span(&mut span); let stmts: SmallVec<_> = noop_flat_map_stmt_kind(kind, vis) .into_iter() .map(|kind| Stmt { id, kind, span }) .collect(); if stmts.len() > 1 { panic!(...); } + vis.visit_span(&mut span); stmts } ```
dtolnay
removed
the
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
label
Jan 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Stmt
case is a bug introduced almost certainly unintentionally by #126993. The code used to visit and mutatespan
correctly, but got changed as follows by that PR. Notice howspan
is copied into the output by|kind| Stmt { id, kind, span }
which happens after the mutation in the correct code (red) and before the mutation in the incorrect code (green).